[test](regression) Add ORC reader V2 Hive coverage#65480
Open
xylaaaaa wants to merge 1 commit into
Open
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
/review |
Contributor
Author
|
run buildall |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Automated review result: request changes due to one test-coverage gap.
Critical checkpoint conclusions:
- Goal/test proof: this PR is intended to add ORC Reader V2 Hive coverage, but the transactional Hive ACID additions do not prove ScannerV2 coverage because those ranges still fall back to the old scanner.
- Scope/focus: the changes are limited to regression tests and are otherwise focused.
- Concurrency/lifecycle/data writes/persistence: no production code paths are changed by this PR.
- Configuration/session propagation: the accepted issue is specifically that setting
enable_file_scanner_v2=trueis insufficient fortransactional_hiveranges because scanner selection rejects that table format. - Parallel paths: transactional Hive has a separate old-scanner reader path, which is the reported coverage gap.
- Test coverage/results: static review only in this runner; the base commit is absent locally,
.worktree_initializedandthirdparty/installedare missing, and external Hive execution is not available here. - User focus: no additional user-provided review focus was present.
Subagent conclusions: optimizer-rewrite reported no optimizer/rewrite candidates. tests-session-config proposed TSC-001, accepted as the inline comment below. Convergence round 1 ended with both live subagents replying NO_NEW_VALUABLE_FINDINGS for the same final ledger/comment set.
xylaaaaa
force-pushed
the
codex/orc-v2-hive-regression
branch
from
July 11, 2026 10:53
eda21cf to
5bbdf3f
Compare
Contributor
|
run buildall |
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Automated review found one blocking issue in the new regression coverage: the Hive ORC predicate assertions force ScannerV2 but rely on a SARG-success session guard that only the legacy ORC reader enforces.
Critical checkpoint conclusions:
- Goal/test proof: the PR is focused on ORC reader V2 Hive regression coverage, but the accepted inline issue means part of that coverage can pass without proving the intended V2 SARG/min-max path.
- Scope: the PR is test-only and otherwise narrow.
- Concurrency, lifecycle, persistence, transactionality, data writes, and FE-BE protocol compatibility: no runtime code is changed directly.
- Configuration/session propagation: the issue is specifically a session-variable coverage gap between old and V2 ORC reader paths.
- Parallel paths: the existing transactional Hive ScannerV2 thread is already known; this review comment covers a distinct non-transactional Hive ORC V2 case.
- Test results and style: GitHub PR patch whitespace validation passed. I did not run regression tests because this checkout lacks
thirdparty/installedand the external Hive environment. - User focus: no additional user-provided review focus was supplied.
Subagent conclusions: OPT-1 and TSC-1 were merged into the single inline comment. A possible profile-token pruning concern was dismissed because FE profile summaries record the original SQL text. Convergence round 1 ended with both optimizer-rewrite and tests-session-config replying NO_NEW_VALUABLE_FINDINGS for the exact final ledger/comment set.
xylaaaaa
force-pushed
the
codex/orc-v2-hive-regression
branch
from
July 12, 2026 04:51
5bbdf3f to
4f97c46
Compare
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29432 ms |
Contributor
TPC-DS: Total hot run time: 180139 ms |
Contributor
ClickBench: Total hot run time: 24.88 s |
Issue Number: None
Related PR: None
Problem Summary: The refactored ORC reader lacked focused Hive external-table regression coverage for SARG conversion across null-safe, integer min/max, decimal, date, and timestamp predicates. It also lacked explicit Reader V2 coverage for lazy materialization result integrity, Hive ACID delete predicates, and virtual row-id planning. Add focused assertions and force FileScannerV2 in the relevant suites while preserving existing generated result files.
None
- Test: Regression test
- Ran ./run-regression-test.sh --run -d external_table_p0/hive -s test_hive_orc_predicate,test_orc_lazy_mat_profile,test_transactional_hive,test_hive_topn_lazy_mat
- Verified all four suites load successfully; Hive external execution was not enabled in the local configuration
- Behavior changed: No
- Does this need documentation: No
xylaaaaa
force-pushed
the
codex/orc-v2-hive-regression
branch
from
July 14, 2026 02:16
4f97c46 to
d954050
Compare
Contributor
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 29397 ms |
Contributor
TPC-DS: Total hot run time: 177380 ms |
Contributor
ClickBench: Total hot run time: 24.92 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
|
/review |
Contributor
There was a problem hiding this comment.
Review complete. I found one blocking test-coverage issue: the ORC lazy-materialization profile suite enables ScannerV2 but does not prove the captured profile actually used ScannerV2, so the intended V2 coverage can pass on the legacy path.
Checkpoint conclusions: the BE V2 SARG guard is small and scoped, the session variable is forwarded through FE to BE, and no separate BE correctness, concurrency, lifecycle, persistence, or protocol-compatibility defect was found. The existing predicate-suite and transactional-Hive review threads were treated as duplicate context and not repeated. Test coverage is the remaining issue: one changed profile suite needs UseScannerV2: true evidence before its ORC counter assertions.
User focus: no additional user-provided review focus was supplied.
Validation: static review only, as this review runner prompt forbids builds or code modifications. The checkout is already at the PR head, so applying the patch forward was not applicable; the reverse patch check against .code-review.xXOf0I/pr.diff succeeded with no whitespace warnings.
| @@ -134,9 +135,14 @@ suite("test_orc_lazy_mat_profile", "p0,external") { | |||
Contributor
There was a problem hiding this comment.
This still does not prove the profile you are checking came from ScannerV2. The suite now sets enable_file_scanner_v2, but all of the counters asserted below (FilteredRowsByLazyRead, EvaluatedRowGroupCount, SelectedRowGroupCount) are also registered by the legacy ORC reader, so the test could keep passing if this scan silently falls back to the old FileScanner path. Please assert the captured profile contains UseScannerV2: true before checking the ORC counters, like the helper added in test_hive_orc_predicate.groovy.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
This PR adds regression coverage for the refactored ORC reader V2 path on Hive external tables. It covers ORC SARG initialization for min/max pruning, null-safe predicates, decimal/date/timestamp predicates, lazy materialization profile metrics, delete predicate behavior on Hive ACID ORC tables, and virtual row id coverage through the existing TopN lazy materialization suite.
Release note
None
Check List (For Author)
./run-regression-test.sh --run -d external_table_p0/hive -s test_hive_orc_predicate,test_orc_lazy_mat_profile,test_transactional_hive,test_hive_topn_lazy_mat./run-regression-test.sh --run -d external_table_p0/hive -s test_hive_orc_predicate,test_orc_lazy_mat_profile./run-regression-test.sh --run -d external_table_p0/hive -s test_orc_lazy_mat_profile